Implement uring in `AsyncWrite for fs::File` #7713

RedKinda · 2025-10-27T14:20:38Z

Motivation

Currently IO operations for fs::File is implemented using spawn_blocking to perform IO, which is not great for an async runtime. This PR implements IO operations using io_uring. The fs::write function already uses io_uring for async operations, and this PR extends the uring functionality into impl AsyncWrite for fs::File.

For full discussion see here #7684 and subsequently on discord https://discord.com/channels/500028886025895936/810724255046172692/1427735341003051079

Solution

The fs::File struct already implements an internal buffer when writing using spawn_blocking, which is similar to what io_uring requires. Therefore in poll_write, we can check if io_uring is initialized and supported, and if not fall back to the spawn_blocking implementation, without requiring any API changes.

This PR only implements AsyncWrite, and once this PR is accepted, I am happy to also open a PR for AsyncRead for File using the same structure.

martin-g · 2025-10-28T15:09:55Z

tokio/src/fs/file.rs

+}
+
+cfg_io_uring! {
+    struct BoxedOp<T>(Pin<Box<dyn Future<Output = T> + Send + Sync + 'static>>);


Does the Future need to be Sync ?
I think it is not really necessary.

Looks like it does need Sync, at least its required by a test https://github.com/RedKinda/tokio/blob/uring_file/tokio/tests/async_send_sync.rs#L651 - https://github.com/tokio-rs/tokio/actions/runs/18909744941/job/53976898682?pr=7713#step:6:248

In principle you could avoid Sync using src/util/sync_wrapper.rs. But for this use-case, it does not matter.

martin-g · 2025-10-28T15:14:46Z

tokio/src/fs/file.rs

+                                    let handle = BoxedOp(Box::pin(async move {
+                                        let (r, buf, _fd) = op.await;
+                                        match r {
+                                            Ok(_n) => (Operation::Write(Ok(())), buf),


I think there is a chance that the buffer is not fully drained here.
Write(Ok()) should be returned only if buf.is_empty().

I think you need to wrap the write in a loop and handle it with something like:

Ok(0) => break (Operation::Write(Err(io::ErrorKind::WriteZero.into())), buf), Ok(_) if buf.is_empty() => break (Operation::Write(Ok(())), buf), Ok(_) => continue, // more to write Err(e) => break (Operation::Write(Err(e)), buf),

Good catch, thanks and fixed

martin-g · 2025-10-28T15:15:44Z

tokio/src/fs/file.rs

+                                    let handle = BoxedOp(Box::pin(async move {
+                                        let (r, buf, _fd) = op.await;
+                                        match r {
+                                            Ok(_n) => (Operation::Write(Ok(())), buf),


Same as above - make sure the buf is empty before saying Ok().

This reverts commit 863a69c.

RedKinda · 2025-11-12T11:34:04Z

This should be ready for merging i think, is there anything else blocking it?

Darksonn · 2025-11-12T11:35:14Z

Sorry I've been too focused on #7696. I will try to review this PR too.

Darksonn · 2025-11-17T19:05:00Z

tokio/src/fs/file.rs

+                        // Handle not present in some tests?
+                        if let Ok(handle) = Handle::try_current() {
+                            if handle.inner.driver().io().check_and_init()? {
+                                task_join_handle = {
+                                    use crate::{io::uring::utils::ArcFd, runtime::driver::op::Op};
+


This is way too many levels of indentation. We need to pull some logic into a separate function.

Darksonn · 2025-11-17T19:06:25Z

tokio/src/fs/file.rs

+#[derive(Debug)]
+enum JoinHandleInner<T> {
+    Blocking(JoinHandle<T>),
+    #[cfg(all(
+        tokio_unstable,
+        feature = "io-uring",
+        feature = "rt",
+        feature = "fs",
+        target_os = "linux"
+    ))]
+    Async(BoxedOp<T>),


Here is an idea:

What if you perform the io-uring logic in a tokio::spawn task? That way, you can use JoinHandle in both cases.

I think it will simply code a lot.

The Blocking(JoinHandle) is crate::blocking::JoinHandle not tokio::runtime::task::join::JoinHandle so some enum would be required anyway, and at that point might as well just have this kind of a boxed future

That is a re-export of the same thing.

tokio/tokio/src/blocking.rs

Line 9 in 4714ca1

pub(crate) use crate::task::JoinHandle;

One reason that I suggest this is that the io-uring logic performs multiple writes in a loop to write the entire buffer, which requires the end-user to await flush or write on the file to make progress. However, the implementation we have today does not require the user to interact with the file to make progress - it happens in the background automatically.

Ah got it, I'll change it.

Out of curiosity, general rule is that for a future to keep making progress, user needs to keep polling it. I feel like not having to do that specifically for fs::File is just an implementation detail, and users shouldn't rely on that always being the case. So i guess my question is how much is it worth preserving old behavior like this when implementing new features, if it comes at the cost of (small) performance overhead? relevant XKCD

Had to adjust mocks to accommodate non-blocking spawning https://github.com/tokio-rs/tokio/actions/runs/19464918802/job/55697554687 b91836c - can you doublecheck im doing it right?

This kind of thing is always a hard question. Though I would say that I'm pretty sure people rely on writes continuing even if the file is dropped, and it would be very hard to change that.

martin-g · 2025-11-18T09:08:28Z

tokio/src/io/uring/write.rs

 }

 impl Op<Write> {
    /// Issue a write that starts at `buf_offset` within `buf` and writes some bytes


This docstring seems to be outdated. buf_offset is no more a parameter

martin-g · 2025-11-18T09:16:32Z

tokio/src/fs/write.rs

        let driver_handle = handle.inner.driver().io();
        if driver_handle.check_and_init()? {
-            return write_uring(path, contents).await;
+            use crate::io::blocking;


Is this import needed here ?
It is already imported at line 8

martin-g · 2025-11-18T09:20:15Z

tokio/src/fs/file.rs

+                                                        buf,
+                                                    );
+                                                }
+                                                Ok(_) if buf.is_empty() => {


I think this arm should be moved to be first, i.e. before the Ok(0) one.

RedKinda added 3 commits October 27, 2025 14:56

implement uring AsyncWrite for File

d9d2ddc

add basic uring File write tests

093070a

formatting

6856207

ADD-SP added C-enhancement Category: A PR with an enhancement or bugfix. A-tokio Area: The main tokio crate M-fs Module: tokio/fs labels Oct 27, 2025

martin-g reviewed Oct 28, 2025

View reviewed changes

RedKinda added 3 commits October 29, 2025 14:30

remove sync bound from BoxedOp

863a69c

make sure to write the full buffer in uring file writes

ea4040d

Revert "remove sync bound from BoxedOp"

226fcf8

This reverts commit 863a69c.

Darksonn reviewed Nov 17, 2025

View reviewed changes

move part of poll_write to its own function

937da2d

martin-g reviewed Nov 18, 2025

View reviewed changes

RedKinda added 5 commits November 18, 2025 12:42

implement review comment suggestions

c30a4dd

add non-blocking spawn to mocks

b91836c

fix type issue in mock spawn

58e9b0b

fix spawn signature 2.0

9504e53

appease clippy

daa0532

Uh oh!

Implement uring in AsyncWrite for fs::File #7713

Are you sure you want to change the base?

Implement uring in AsyncWrite for fs::File #7713

Conversation

RedKinda commented Oct 27, 2025

Motivation

Solution

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RedKinda Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RedKinda commented Nov 12, 2025

Uh oh!

Darksonn commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implement uring in `AsyncWrite for fs::File` #7713

Implement uring in `AsyncWrite for fs::File` #7713

RedKinda Oct 29, 2025 •

edited

Loading